Reservations RP 2018-06-01: Add CosmosDb in reserved resource enum type. Add name property in PatchProperties by luluRagdoll · Pull Request #7164 · Azure/azure-powershell

luluRagdoll · 2018-09-07T22:21:23Z

Description

Add CosmosDb in reserved resource enum type.
Add name property in PatchProperties

Checklist

I have read the Submitting Changes section of CONTRIBUTING.md
The title of the PR is clear and informative
The appropriate change log has been updated
The PR does not introduce breaking changes
If applicable, the changes made in the PR have proper test coverage
For public API changes to cmdlets:
- the changes have gone through a cmdlet design review
- the cmdlet markdown files were generated using the platyPS module

…y in the PatchProperties

…nto preview

luluRagdoll · 2018-09-08T00:15:07Z

@cormacpayne Hi Cormac,
Since the .NET SDK Microsoft.Azure.Management.Reservations 1.9.0-preview version is not published, I added in the localfeed to get unblocked.
The change is to add CosmosDb as a new type in the reserved resource enum type.
Add name property in the Patch properties.

Could you help take a look?Thx

maddieclayton · 2018-09-11T18:14:18Z

@luluRagdoll please link your cmdlet design review from here: https://github.com/Azure/azure-powershell-cmdlet-review-pr

luluRagdoll · 2018-09-11T18:18:37Z

@maddieclayton Sure. I will work on that and link it. :)

luluRagdoll · 2018-09-11T19:03:18Z

@maddieclayton This is the design review link: https://github.com/Azure/azure-powershell-cmdlet-review-pr/issues/154 Please feel free to let me know if you have any questions. Thanks

MiYanni

You need a ChangeLog.md entry for Reservations for these changes to be accepted.
You are missing scenario tests using Name and the new value for ReservedResourceType.

MiYanni · 2018-09-11T21:24:29Z

src/ResourceManager/Reservations/Commands.Reservations/help/Get-AzureRmReservation.md


 ```yaml
-Type: Microsoft.Azure.Commands.Common.Authentication.Abstractions.IAzureContextContainer
+Type: IAzureContextContainer


You need to regenerate all the help files using the UseFullTypeName flag. Details here: https://github.com/Azure/azure-powershell/blob/preview/documentation/development-docs/help-generation.md#updating-all-markdown-files-in-a-module

@MiYanni

You need a ChangeLog.md entry for Reservations for these changes to be accepted. --- Seems we didn't put anything in the file before.
Can I know what should be the version and what should be the date?
The latest one in the ChangeLog.md is "## 6.8.1 - August 2018"

Yes. I think I need to update the scenario test using Name and new reservedResourceType.

In your changelog, src\ResourceManager\Reservations\ChangeLog.md, you add an entry under the Current Release header.

Sounds good.

Thanks. DONE.

You didn't regenerate the help documentation using the UseFullTypeName flag.

MiYanni · 2018-09-11T21:27:27Z

src/ResourceManager/Reservations/Commands.Reservations/Cmdlets/PatchReservation.cs


+        [Parameter(Mandatory = false)]
+        [ValidateNotNull]
+        public string Name { get; set; }


I don't see updated help documentation for the Name parameter. Also, please provide an example in the help using this parameter.

src/ResourceManager/Reservations/Commands.Reservations/Cmdlets/GetCatalog.cs

maddieclayton · 2018-09-12T17:45:31Z

@MiYanni this will need a new branch for preview release.

MiYanni · 2018-09-12T18:54:23Z

@maddieclayton I'll create a branch when this is ready for merging. As it stands, the design review hasn't been accepted yet.

src/ResourceManager/Reservations/Commands.Reservations/Cmdlets/GetCatalog.cs

MiYanni · 2018-09-14T20:17:47Z

src/ResourceManager/Reservations/Commands.Reservations/help/Get-AzureRmReservation.md


 ```yaml
-Type: Microsoft.Azure.Commands.Common.Authentication.Abstractions.IAzureContextContainer
+Type: IAzureContextContainer


You didn't regenerate the help documentation using the UseFullTypeName flag.

luluRagdoll · 2018-09-14T21:13:30Z

I see. Looks like I should not use the command that our team has in the doc. Will follow https://github.com/Azure/azure-powershell/blob/preview/documentation/development-docs/help-generation.md#updating-all-markdown-files-in-a-module to get the full name.

luluRagdoll · 2018-09-14T21:22:12Z

@MiYanni I just updated the full name in the help file and updated our own one note. Thanks for pointing out. It looks much better. Please feel free to let me know if there is anything else.

MiYanni · 2018-09-14T21:52:31Z

src/ResourceManager/Reservations/Commands.Reservations/ChangeLog.md

        - Additional information about change #1
 -->
 ## Current Release
+## Version 0.1.7


Please remove the ## Version 0.1.7 tag. Just keep the entries under the ## Current Release tag.

Sure. Done.

MiYanni

LGTM

MiYanni · 2018-09-14T22:54:50Z

@luluRagdoll
I've accepted the PR. However, the build failed. Please look into the details and fix the issues.

luluRagdoll · 2018-09-14T23:08:20Z

Sure. Taking a look now.

MiYanni · 2018-09-19T18:19:36Z

@luluRagdoll Any update? It looks like 1 test is failing, TestGetCatalog.

luluRagdoll · 2018-09-19T19:40:29Z

Hi Michael, I have root cause the issue. Will update this today. Thanks Yulu

…

Sent from my iPhone

On Sep 19, 2018, at 11:23 AM, Michael Yanni ***@***.***> wrote: @luluRagdoll Any update? It looks like 1 test is failing, TestGetCatalog. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or mute the thread.

MiYanni · 2018-09-20T17:56:22Z

src/ResourceManager/Reservations/Commands.Reservations/help/Update-AzureRmReservation.md

 ```

+### -Name
+{{Fill Name Description}}


This is missing a description for this parameter.

Got it. Making the change now. :)

MiYanni · 2018-09-20T17:57:26Z

@luluRagdoll Build is passing, but just a minor change to the help documentation is needed. If you make this change, I can release the module to the test gallery today.

MiYanni · 2018-09-20T22:05:00Z

demand: https://azuresdkci.westus2.cloudapp.azure.com/view/PowerShell/job/powershell-demand/1111/
ps-sign: https://azuresdkci.westus2.cloudapp.azure.com/view/PowerShell/job/ps-sign/1369/
psnetsign: https://azuresdkci.westus2.cloudapp.azure.com/view/PowerShell/job/psnetsign/212/

dorislewisbjtu added 3 commits September 6, 2018 10:23

Add new CosmosDb in the reserved resource type enum. Add name propert…

e3360fc

…y in the PatchProperties

Merge branch 'preview' of https://github.com/Azure/azure-powershell i…

fc60fa6

…nto preview

Add name in Patch Properties

68a2f63

azuresdkci1x added the needs-review label Sep 7, 2018

luluRagdoll requested a review from cormacpayne September 8, 2018 00:13

luluRagdoll requested a review from maddieclayton September 10, 2018 18:16

Merge branch 'preview' into reservations

05c123b

cormacpayne self-assigned this Sep 11, 2018

maddieclayton mentioned this pull request Sep 11, 2018

Reservations RP 2018-06-01: Add CosmosDb in reserved resource enum type. Add name property in PatchProperties #7126

Closed

8 tasks

maddieclayton added the Cmdlet Review Required ⚠️ label Sep 11, 2018

maddieclayton added the Preview label Sep 11, 2018

maddieclayton assigned MiYanni and unassigned cormacpayne Sep 11, 2018

MiYanni suggested changes Sep 11, 2018

View reviewed changes

MiYanni assigned luluRagdoll and unassigned MiYanni Sep 11, 2018

MiYanni added needs-revision and removed needs-review labels Sep 11, 2018

maddieclayton assigned MiYanni Sep 11, 2018

MiYanni removed their assignment Sep 11, 2018

maddieclayton removed the needs-revision label Sep 12, 2018

maddieclayton assigned MiYanni Sep 12, 2018

MiYanni added the needs-revision label Sep 12, 2018

Address comments

1c47af1

MiYanni suggested changes Sep 14, 2018

View reviewed changes

Update help file to full type name

fd023fa

MiYanni suggested changes Sep 14, 2018

View reviewed changes

Modify change log

93b68af

MiYanni added Cmdlet Review Required ⚠️ Cmdlet Review Complete and removed Cmdlet Review Required ⚠️ labels Sep 14, 2018

MiYanni previously approved these changes Sep 14, 2018

View reviewed changes

Merge branch 'AzureRM.Reservations-preview' into reservations

cab9f49

MiYanni dismissed their stale review via cab9f49 September 17, 2018 19:19

MiYanni changed the base branch from preview to AzureRM.Reservations-preview September 17, 2018 19:20

Fix test error

aee3ade

MiYanni suggested changes Sep 20, 2018

View reviewed changes

dorislewisbjtu added 2 commits September 20, 2018 14:53

Add help msg

1e48834

Manually add name property description

fb55009

MiYanni approved these changes Sep 20, 2018

View reviewed changes

MiYanni added Waiting for CI and removed needs-revision labels Sep 20, 2018

MiYanni unassigned luluRagdoll Sep 20, 2018

MiYanni merged commit c94afe2 into AzureRM.Reservations-preview Sep 21, 2018

maddieclayton deleted the reservations branch October 1, 2018 21:27

Conversation

luluRagdoll commented Sep 7, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Checklist

Uh oh!

luluRagdoll commented Sep 8, 2018

Uh oh!

maddieclayton commented Sep 11, 2018

Uh oh!

luluRagdoll commented Sep 11, 2018

Uh oh!

luluRagdoll commented Sep 11, 2018

Uh oh!

MiYanni left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

luluRagdoll Sep 14, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

maddieclayton commented Sep 12, 2018

Uh oh!

MiYanni commented Sep 12, 2018

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

luluRagdoll commented Sep 14, 2018

Uh oh!

luluRagdoll commented Sep 14, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

MiYanni left a comment

Choose a reason for hiding this comment

Uh oh!

MiYanni commented Sep 14, 2018

Uh oh!

luluRagdoll commented Sep 14, 2018

Uh oh!

MiYanni commented Sep 19, 2018

Uh oh!

luluRagdoll commented Sep 19, 2018 via email

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

MiYanni commented Sep 20, 2018

Uh oh!

MiYanni commented Sep 20, 2018

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

luluRagdoll commented Sep 7, 2018 •

edited

Loading

luluRagdoll Sep 14, 2018 •

edited

Loading